Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Refactor to Use pathlib.Path for Path-Related Variables #177

Closed
wants to merge 3 commits into from
Closed

Refactor to Use pathlib.Path for Path-Related Variables #177

wants to merge 3 commits into from

Conversation

TaekyungHeo
Copy link
Member

@TaekyungHeo TaekyungHeo commented Aug 19, 2024 •
edited
Loading

Summary

Refactor to use pathlib.Path for path-related variables. @amaslenn let me know that using the Path type is a better practice in Python for path-related variables. This PR replaces str with pathlib.Path for path-related variables.

Test Plan

1. CI
CI passes
2. Ran on a server
Sleep

$ python cloudaix.py --mode run --system-config conf/v0.6/general/system/system.toml --test-templates-dir conf/v0.6/general/test_template --tests-dir conf/v0.6/general/test --test-scenario conf/v0.6/general/test_scenario/sleep.toml 
TYPE <class 'NoneType'>
[INFO] System configuration file: conf/v0.6/general/system/system.toml
[INFO] Test templates directory: conf/v0.6/general/test_template
[INFO] Tests directory: conf/v0.6/general/test
[INFO] Test scenario file: conf/v0.6/general/test_scenario/sleep.toml
[INFO] Output directory: None
[INFO] System Name: System
[INFO] Scheduler: slurm
[INFO] Test Scenario Name: test_scenario_example
[INFO] Checking if test templates are installed.
[INFO] Test Scenario: test_scenario_example

Section Name: Tests.1
  Test Name: sleep_10
  Description: sleep_10
  No dependencies
Section Name: Tests.2
  Test Name: sleep_5
  Description: sleep_5
  Start Post Init: Tests.1, Time: 5 seconds
Section Name: Tests.3
  Test Name: sleep_5
  Description: sleep_5
  Start Post Comp: Tests.1, Time: 0 seconds
Section Name: Tests.4
  Test Name: sleep_20
  Description: sleep_20
  End Post Comp: Tests.1, Time: 5 seconds
[INFO] Initializing Runner
[INFO] Creating SlurmRunner
[INFO] Starting test scenario execution.
[INFO] Starting test: Tests.1
[INFO] Running test: Tests.1
[INFO] Executing command for test Tests.1: sbatch /obfuscated/path/results/test_scenario_example_2024-08-23_23-12-50/Tests.1/0/cloudai_sbatch_script.sh
[INFO] Starting test: Tests.4
[INFO] Running test: Tests.4
[INFO] Executing command for test Tests.4: sbatch /obfuscated/path/results/test_scenario_example_2024-08-23_23-12-50/Tests.4/0/cloudai_sbatch_script.sh
[INFO] Delayed start for test Tests.2 by 5 seconds.
[INFO] Starting test: Tests.2
[INFO] Running test: Tests.2
[INFO] Executing command for test Tests.2: sbatch /obfuscated/path/results/test_scenario_example_2024-08-23_23-12-50/Tests.2/0/cloudai_sbatch_script.sh
[INFO] Job completed: Tests.1
[INFO] Delayed start for test Tests.3 by 0 seconds.
[INFO] Starting test: Tests.3
[INFO] Running test: Tests.3
[INFO] Executing command for test Tests.3: sbatch /obfuscated/path/results/test_scenario_example_2024-08-23_23-12-50/Tests.3/0/cloudai_sbatch_script.sh
[INFO] Scheduling termination of job 286821 after 5 seconds.
[INFO] Job completed: Tests.2
[INFO] Job completed: Tests.4
[INFO] Job completed: Tests.3
[INFO] All test scenario results stored at: /obfuscated/path/results/test_scenario_example_2024-08-23_23-12-50
[WARNING] Skipping directory '/obfuscated/path/results/test_scenario_example_2024-08-23_23-12-50/Tests.1/0' for test 'sleep_10'
[WARNING] Skipping directory '/obfuscated/path/results/test_scenario_example_2024-08-23_23-12-50/Tests.2/0' for test 'sleep_5'
[WARNING] Skipping directory '/obfuscated/path/results/test_scenario_example_2024-08-23_23-12-50/Tests.3/0' for test 'sleep_5'
[WARNING] Skipping directory '/obfuscated/path/results/test_scenario_example_2024-08-23_23-12-50/Tests.4/0' for test 'sleep_20'

NCCL tests

$ python cloudaix.py --mode run --system-config conf/v0.6/general/system/system.toml --test-templates-dir conf/v0.6/general/test_template --tests-dir conf/v0.6/general/test --test-scenario conf/v0.6/general/test_scenario/nccl_test.toml 
[INFO] System configuration file: conf/v0.6/general/system/system.toml
[INFO] Test templates directory: conf/v0.6/general/test_template
[INFO] Tests directory: conf/v0.6/general/test
[INFO] Test scenario file: conf/v0.6/general/test_scenario/nccl_test.toml
[INFO] Output directory: None
[INFO] System Name: System
[INFO] Scheduler: slurm
[INFO] Test Scenario Name: nccl-test
[INFO] Checking if test templates are installed.
[INFO] Test Scenario: nccl-test

Section Name: Tests.1
  Test Name: nccl_test_all_reduce
  Description: all_reduce
  No dependencies
[INFO] Initializing Runner
[INFO] Creating SlurmRunner
[INFO] Starting test scenario execution.
[INFO] Starting test: Tests.1
[INFO] Running test: Tests.1
[INFO] Executing command for test Tests.1: sbatch /obfuscated/path/results/nccl-test_2024-08-23_23-41-44/Tests.1/0/cloudai_sbatch_script.sh
[INFO] Job completed: Tests.1

@TaekyungHeo TaekyungHeo changed the title Refactor path Use Path for Path-related Variables Aug 19, 2024
@TaekyungHeo TaekyungHeo force-pushed the refactor-path branch 2 times, most recently from 516254c to fcb65d4 Compare August 20, 2024 13:16
@TaekyungHeo TaekyungHeo changed the title Use Path for Path-related Variables Refactor to use pathlib.Path for path-related variables Aug 20, 2024
@TaekyungHeo TaekyungHeo changed the title Refactor to use pathlib.Path for path-related variables Refactor to Use pathlib.Path for Path-Related Variables. Aug 20, 2024
@TaekyungHeo TaekyungHeo force-pushed the refactor-path branch 2 times, most recently from a90a440 to c5fd63f Compare August 20, 2024 13:23
@TaekyungHeo TaekyungHeo added the Oct24 Oct'24 release feature label Aug 20, 2024
@TaekyungHeo TaekyungHeo force-pushed the refactor-path branch 2 times, most recently from f62b513 to 1917e69 Compare August 20, 2024 19:19
@TaekyungHeo TaekyungHeo changed the title Refactor to Use pathlib.Path for Path-Related Variables. Refactor to Use pathlib.Path for Path-Related Variables Aug 20, 2024
@TaekyungHeo TaekyungHeo force-pushed the refactor-path branch 2 times, most recently from 71d22c2 to 146d260 Compare August 21, 2024 20:51
@TaekyungHeo TaekyungHeo marked this pull request as ready for review August 21, 2024 20:51
@TaekyungHeo TaekyungHeo marked this pull request as draft August 21, 2024 20:53
@TaekyungHeo TaekyungHeo mentioned this pull request Aug 21, 2024
Merged
@TaekyungHeo TaekyungHeo force-pushed the refactor-path branch 4 times, most recently from 7263372 to 7e1479b Compare August 23, 2024 20:27
@TaekyungHeo TaekyungHeo marked this pull request as ready for review August 23, 2024 20:47
@amaslenn
Copy link
Contributor

@TaekyungHeo if possible, let's make this PR independent from others, because this one is easier to accept and merge, otherwise it might be blocked by others.

@TaekyungHeo
Copy link
Member Author

@amaslenn , I created a new PR here: #183

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@amaslenn amaslenn Awaiting requested review from amaslenn

@artemry-nv artemry-nv Awaiting requested review from artemry-nv

@srivatsankrishnan srivatsankrishnan Awaiting requested review from srivatsankrishnan

@srinivas212 srinivas212 Awaiting requested review from srinivas212

Assignees
No one assigned
Labels
Oct24 Oct'24 release feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@TaekyungHeo @amaslenn